Write keeps caller-owned changesets and serializes them later. Because Write returns after enqueueing, callers can mutate or reuse cs before serializerLoop runs, causing the WAL to persist changed data or race with the caller

This pattern is intentional for performance, but not well documented.

I updated all godocs to say that it is unsafe to modify slices passed into methods or slices received from methods.

End block before append succeeds

High Severity

SignalEndOfBlock sets currentBlockEnded and clears buf before wal.Append succeeds. If scheduling fails, the block is treated as finalized while its changesets were never queued, so further writes to that block are rejected and a later write to the next block number is allowed—silently skipping the lost block.

 

^{Reviewed by Cursor Bugbot for commit d9a978b. Configure here.}

the StateWAL now bricks itself on the first error, making this scenario no longer a problem.

Shall we consider making it goroutine concurrency safe then? It would things much easier from user perspective. My concern is if we implement a non thread safe library, it might makes future integration error prune especially when someone who is not familiar with the library.

I could also imagine sometimes we have to pass WAL around to different objects and call functions in different goroutines, it might be hard to always serialize the function calls of the WAL for both reads and writes.

But if the current usage doesn't require any concurrency call, then I think I'm good for the first pass and we can add that later when we really have concurrency usage

What's the point of allowing Index to not be contiguous here?
Is there any scenario that we want to do:
append(1, xxx), append(3, xxx), append(4, xxx), append(10, xxx)

On the other hand, do we really need the caller of this API to pass in the index? If the write pattern is always append only, I wonder if we can just internally manage this index and auto increment it? The benefit is that the caller doesn't need to think about how to get and increment the index any more during writes. So the suggested API would look like this:

Append(data T) error

This Prune (recommend to rename to PruneBefore) is equivalent to truncate from first to lowestIndexToKeep.
I think we need another API PruneAfter() that is equivalent to truncate from lastIndexToKeep to last.

One use case I think think of PruneAfter useful is for mannual rollback

Personally I would recommend to replace this API or add another API for Replay(startIndex, endIdex) to match our existing usage pattern.

The iterator exists to support streaming reads concurrent with ongoing writes and pruning. That's why it needs all the machinery:

• Sealing the mutable file — snapshot the write frontier so the iterator sees a consistent point-in-time view
• Pinning (indexRefs) — prevent Prune() from deleting files the iterator hasn't read yet
• Prefetch goroutine — read ahead so the consumer isn't blocked on I/O
• Close lifecycle — unpin so pruning can resume

The core question is: does the WAL actually need streaming reads concurrent with writes and pruning? How a WAL is typically used in a blockchain node?

In practice, WAL replay happens in two scenarios:

1. Startup recovery — replay all records to rebuild uncommitted state. This happens before new writes begin. No concurrency with appends or pruning.
2. Periodic checkpointing — read records since last checkpoint, apply them, then prune. This could be concurrent with new appends, but pruning only removes records before the read range, not within it.

In neither case do you need a long-lived iterator that coexists with concurrent writes and pruning to the same range. You just need: "give me all records from index X to the end."

What a replay function would look like? The simplest version:
func (w *walImpl) Replay(startIndex uint64, fn func(index uint64, data []byte) error) error
Implementation-wise, you'd send a replay request through the writer channel (so it's ordered relative to appends). The writer goroutine would:

1. Flush (not seal) the mutable file
2. Collect the list of sealed files + the mutable file path and its current size
3. Return that snapshot to the caller

Then the caller reads through those files sequentially, calling fn for each record. No background goroutine, no pinning, no lifecycle management.
The pruning concern vanishes for two reasons. On the pruning-during-replay race — you have two clean options:

Option A (simplest): Open all relevant file handles upfront before returning from the writer goroutine. On Linux/macOS, an open file handle survives unlink, so even if Prune() deletes the file from the directory, the handle keeps the data accessible. No pinning needed at all.

Option B: Just document that the caller must not prune indices it's replaying. Since the caller controls both replay and prune, this is trivially enforceable — you prune after replay completes, not during.

What you'd delete? With a replay function, you can remove:

• walIterator struct and the entire seiwal_iterator.go file
• The prefetch goroutine, stop channel, readerExited channel
• indexRefs map and the entire pinning mechanism (pinLowestReadableIndex, unpinIndex, lowestReservation)
• sealForIterator — no more forced rotation on reads
• The iteratorRequest message type in the writer channel
• The it.done data race issue
• The IteratorPrefetchSize config field

That's a substantial reduction in surface area. The core WAL (append, flush, seal, prune, recover) stays the same.

What you'd lose
The only thing you give up is streaming reads with bounded memory during concurrent operation. Specifically:

If you use Option A (open handles upfront), you hold file descriptors for the duration of replay. With many sealed files this could be a lot of FDs, but in practice the count is small.

If the replay callback is slow, new appends continue normally (the mutable file keeps growing), but the replayed snapshot doesn't include them. This is fine — replay is point-in-time by definition.
You lose the ability to do lazy/partial reads (stop iteration early). But with a callback, fn can return a sentinel error to stop early — same effect, simpler mechanism.

Startup validates every sealed file by reading it entirely into memory which could be very slow and could evict page cache which could impact performance.

readWalFile does this:
data, err := os.ReadFile(path)

It is not uncommon that the WAL files could be accumulated into GB or even TB large, at that scale, starting up could take minutes/hours to finish the scanning

The sealed files already have per-record CRCs baked into the format. So any corruption — bit-rot, truncation, garbled bytes — will be caught when that record is actually read during replay. The startup validation is doing the exact same CRC checks again, ahead of time, for records you may never even read (if they get pruned before the next replay).

And reading GB of WAL files at startup has a second nasty effect: it floods the OS page cache with cold WAL data, evicting hot pages from the actual database (memiavl, flatKV, pebble, etc.). The first minutes of operation after startup become slow as the real working set faults back in from disk.

Recommendation: three tiers of startup validation
Tier 1 — Metadata-only (default, microseconds)
At startup, only stat() each sealed file. Verify structural invariants without reading any file contents. This catches: missing files, zero-length/truncated files, naming inconsistencies, index gaps. Cost is one stat syscall per file — microseconds even with thousands of files.

Tier 2 — Bookend validation (optional, milliseconds)
Read only the header + first record + last record of each file. This catches filename-content mismatches without reading the interior.
Cost: two small reads per file (first few bytes + last ~1MB scanned for the final record). For 1000 files this is still sub-second.

Tier 3 — Full validation (optional, background)
Keep the current full-read CRC validation but run it after startup, in a background goroutine, and use fadvise to avoid polluting the page cache.

Not a big fan of adding rolbackIndex to the constructor, I mean it's fine to keep, but we should also support ondemand rollbacks, not just auto rollback during initialization.

The inner walImpl.Append enforces strict index ordering via appendMu, but serializingWAL.Append does not. Since serialization is deferred to a background goroutine, two rapid calls with indices (5, 3) would both be accepted and enqueued. The inner WAL catches the out-of-order index later, but that fires asynchronously — calling w.fail() which bricks the WAL. The caller of serializingWAL.Append(3, ...) already received a nil error and believes the append succeeded.

There's no appendMu equivalent to catch the misordering synchronously. Either mirror the appendMu gate here, or clearly document that the serializing wrapper provides strictly weaker ordering guarantees than the byte WAL.

The closure func() ([]byte, error) { return s.serialize(data) } captures data by value. For T = SomeStruct with slice fields, or T = []byte, the caller could mutate the underlying backing array between Append returning and the serializer goroutine running. The byte-oriented walImpl.Append is safer because it calls frameRecord(index, data) synchronously, copying the payload before enqueueing.

The doc already warns about this, but the asymmetry between the two WAL flavors (byte WAL copies eagerly, serializing WAL defers) is a sharp edge. Consider serializing eagerly on the caller's goroutine and enqueueing the already-serialized bytes, then it would match walImpl's behavior and also let you validate index ordering synchronously.

Recommend to use atomic.Bool defensively — it costs nothing and prevents a real data race.
done is read by Next without any synchronization, so there could be data race if Close and Next overlap

Would recommend default to false though for performance concern

an interrupted rollback is not resumed on a plain reopen, and the contract doesn't say so